-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[5889] Bmoric/log application name for all workers [2/2] #7268
Conversation
…airbyte into bmoric/add-application-name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. Will wait to do a full review until the scope of application logging is solidified.
…rbyte into bmoric/log-application-name
…airbyte into bmoric/add-application-name
…rbyte into bmoric/log-application-name
Just a random idea. could we inject the name through a property instead and just have the log4j pattern to use that property? |
@michel-tricot , This is actually what we do here, I splitted the review into 3 different ones. The first one is doing what you are suggesting. With the removal the log in the Source and destination for example, it makes less sense to have the complicated system. My original thought was to be able to have something like:
So even if you're runningin the same application, you know exactly where the log comes from. The main idea was also to have a system in place to be able to add more information in the logs. |
@@ -28,6 +31,10 @@ | |||
|
|||
private static final Logger LOGGER = LoggerFactory.getLogger(DbtTransformationRunner.class); | |||
private static final String DBT_ENTRYPOINT_SH = "entrypoint.sh"; | |||
private static final MdcScope CONTAINER_LOG_MDC = new Builder() | |||
.setLogPrefix("dbt-container-log") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.setLogPrefix("dbt-container-log") | |
.setLogPrefix("dbt") |
@@ -27,6 +30,10 @@ | |||
public class DefaultNormalizationRunner implements NormalizationRunner { | |||
|
|||
private static final Logger LOGGER = LoggerFactory.getLogger(DefaultNormalizationRunner.class); | |||
private static final MdcScope CONTAINER_LOG_MDC = new Builder() | |||
.setLogPrefix("normalization-container-log") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.setLogPrefix("normalization-container-log") | |
.setLogPrefix("normalization") |
@@ -30,6 +33,10 @@ | |||
public class DefaultAirbyteDestination implements AirbyteDestination { | |||
|
|||
private static final Logger LOGGER = LoggerFactory.getLogger(DefaultAirbyteDestination.class); | |||
private static final MdcScope CONTAINER_LOG_MDC = new Builder() | |||
.setLogPrefix("destination-container-log") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.setLogPrefix("destination-container-log") | |
.setLogPrefix("destination") |
@@ -35,6 +38,11 @@ | |||
private static final Duration GRACEFUL_SHUTDOWN_DURATION = Duration.of(10, ChronoUnit.HOURS); | |||
private static final Duration FORCED_SHUTDOWN_DURATION = Duration.of(1, ChronoUnit.MINUTES); | |||
|
|||
private static final MdcScope CONTAINER_LOG_MDC = new Builder() | |||
.setLogPrefix("source-container-log") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.setLogPrefix("source-container-log") | |
.setLogPrefix("source") |
) This is adding the application name in the logs produced by the docker containers. This is related to airbytehq#5889.
What
This is adding the application name in the logs produced by the docker containers.
This is related to #5889.